Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Github CI #19

Merged
merged 8 commits into from
Aug 4, 2024
Merged

Add Github CI #19

merged 8 commits into from
Aug 4, 2024

Conversation

DLochmelis33
Copy link
Collaborator

The current CI fails on MacOS because of the new UPUB bug

@DLochmelis33 DLochmelis33 requested a review from eupp July 7, 2024 17:20
- name: Run litmus tests via CLI
run: ./cli/build/bin/linuxX64/releaseExecutable/cli.kexe -r pthread ".*"
- name: Run a single test with JCStress
run: ./gradlew :cli:jvmRun --args="-r jcstress -j '-m quick' StoreBuffering.Plain"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not run all jcstress tests?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed this on a meeting.

The problem is that apparently there is no option in jcstress to control the duration of testing, except the -m (mode) option. But, event with -m quick all the tests that we have now are run ~30mins, which is too long.

The purpose of our local CI is to run "smoke-tests" of the litmuskt tool itself (i.e. to quickly check the basic functionality of the tool), and not to exhaustively test the Kotlin compiler.
Therefore, it was decided to run just a single jcstress test on CI, only to check that the jcstress integration works.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DLochmelis33 could you please add the info from the comment above to the code.
After this we can merge this PR.

@DLochmelis33
Copy link
Collaborator Author

Added the comment, decided to let JCStress run all tests, found a couple of bugs, fixed them

@DLochmelis33 DLochmelis33 merged commit 747d314 into development Aug 4, 2024
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants